Skip to content

Conversation

@antoyo
Copy link

@antoyo antoyo commented Apr 5, 2025

This fixes an infinite recursion in sqrt for codegen backends that implement intrinsics like simd_fsqrt by calling sqrt on every element of the vector.

Fix #649

This requires rust-lang/libm#532 to fix the issue, so I'll need to update the submodule when this PR is merged.

This fixes an infinite recursion in sqrt for codegen backends that
implement intrinsics like simd_fsqrt by calling sqrt on every element of
the vector.
@antoyo antoyo marked this pull request as draft April 5, 2025 14:04
@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

Sorry about the regression here. Could this be temporally fixed by setting the compiler-builtins-no-asm feature when building with cg_gcc https://github.com/rust-lang/rust/blob/d4f880f8ce832cd7560bb2f1ebc34f967055ffd7/library/std/Cargo.toml#L99? I think bootstrap has a way to configure this.

The change in libm was made to move away from subtractive features like force-soft-float because that doesn't work well with Cargo and config was getting complicated (now gone in libm but still exists in compiler-builtins). If something like this is needed, I'd prefer splitting out separate features to enable assembly and to enable the rest of core::arch, rather than adding back force-soft-float. I can do this work, just let me know if no-asm is an option.

@antoyo
Copy link
Author

antoyo commented Apr 9, 2025

just let me know if no-asm is an option.

Just to make sure, this will disable some optimizations, right?
If so, I'd prefer to not use no-asm since rustc_codegen_gcc supports inline assembly.

@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

Yeah you're right, that would affect compiler-builtins. It's not a problem in libm since there are no uses of asm! though.

So what might be best is to comment out println!("cargo:rustc-cfg=arch_enabled"); with a FIXME to add this back in the future once libm drops all core::arch calls. This is the plan after the functions are in core (I'm hoping rust-lang/rust#138087 can go to stable after only a couple of cycles).

The existing mention of force-soft-floats can be deleted too, it isn't used in either crate anymore so I guess I forgot to clean it up.

@antoyo
Copy link
Author

antoyo commented Apr 9, 2025

So what might be best is to comment out println!("cargo:rustc-cfg=arch_enabled"); with a FIXME to add this back in the future once libm drops all core::arch calls.

Ok, do you want me to make a PR for this or will you do it?
I might need more details if you want me to make the PR.

Thanks!

@tgross35
Copy link
Contributor

tgross35 commented Apr 9, 2025

You can probably just update this PR, something like:

    // The arch module may contain assembly.
    if !cfg!(feature = "no-asm") {
        // FIXME: when part of builtins, `libm` can use assembly but cannot use vector intrinsics
        // from `core::arch` in order to avoid recursion problems. Once basic `libm` functions
        // are available in `core`, we will be able to remove these problematic calls and add back
        // `arch_enabled`.
        // Cc: <https://github.com/rust-lang/compiler-builtins/issues/649>
        // println!("cargo:rustc-cfg=arch_enabled");
    }

@antoyo
Copy link
Author

antoyo commented Apr 18, 2025

Closing since this was fixed elsewhere.

@antoyo antoyo closed this Apr 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infinite recursion in sqrt

2 participants